This behaved in a way that was unintuitive due to the fact that it used the total render count rather than the iterator in its implementation. This isn't currently used, so I'd like us to try using the other methods first before determining if we need to reintroduce this.

If we do reintroduce, I'd propose that instead of thinking about taking each render up to a specific count, that instead this method advances the iterator a certain number of renders from "current". Doing so would prevent weird situations like this:

await Profiled.takeRender()
await Profiled.takeRender()
await Profiled.takeRender()

// This makes no sense since render #2 is now in the past
await Profiled.takeUntilRenderCount(2)

This is now the only function that doesn't rely on the iterator position in its implementation. If possible, we should keep it that way to try and maintain the intent of the original API which should force you to iterate on each render in your tests.

I added a setSnapshot method which allows us now to set a partial snapshot, which gets merged with the full snapshot. I took the name from React's old this.setState() method in class components, which has similar behavior.

See the comment above - we might also consider the current behaviour, but immer-wrapped.

Because I'm changing "current render" to mean the render pointing to the most recent takeRender call, I'm requiring that takeRender be called at least once. We could just point at the first render if its available, but I felt that to be weird in the following situation:

const current1 = Profiled.getCurrentRender();
const result = await Profiled.takeRender();

// Without requiring `takeRender`, this would be equal to current1
const current2 = Profiled.getCurrentRender();

On the flip side, we could make getCurrentRender be equal to the iterator position, but this means that it represents the render "after" the last takeRender call, which also feels weird. This means that calling getCurrentRender after you've exhausted the render array via takeRender would point to a render that does not exist.

I'm almost considering that we get immer in here for the tests.

That would end up looking like this,

but would still enable something like this

            ProfiledApp.updateSnapshot(snapshot => {
              return {}
            })

which would reset the snapshot to an empty object - which would not be possible with the setSnapshot notation here.

I guess I'm indifferent here. I didn't consider an alternative library only because it was a small change, but I can see how it would be useful.

FWIW, to reset back to an empty object, I figured you'd just use updateSnapshot directly since updateSnapshot does a whole replacement on the snapshot object.

ProfiledApp.updateSnapshot({})

I never intended setSnapshot to be used for this case, and IMO was the differentiator between the update* and set* versions of this function.

The reason I felt like a setSnapshot would be useful (which you don't see in this PR since this PR contains updates to counts, but would be used in useLoadableQuery) is cases where you want to record the value from a hook, which might only be part of the whole snapshot.

function Child({ queryRef }) {
  const { data } = useReadQuery(queryRef)

  // No need for the function callback since I can just update the partial snapshot
  ProfiledApp.setSnapshot({ data })
}

// or record the whole object under a specific key without the need for a callback function
ProfiledApp.setSnapshot({ result: useReadQuery(queryRef) })

Again, I'm indifferent, and if adding a dependency on immer would make this more useful, thats fine.

Thoughts?

Actually, I'm just now realizing that produce in Immer enforces a callback function.

The motivation behind setSnapshot was to try and avoid the noise of a callback function when I just wanted to record a new value for a particular key. This PR doesn't do a great job of highlighting this use case since updateSnapshot is mainly used to increment counters, but there are a decent number of cases in the useLoadableQuery PR that would benefit from the setSnapshot style.

const result = useReadQuery(queryRef)

ProfiledApp.setSnapshot({ result })

// vs
ProfiledApp.updateSnapshot((snapshot) => ({ ...snapshot, result }))

// or with immer
ProfiledApp.updateSnapshot((snapshot) => {
  snapshot.result = result;
})

This is especially useful if you just want to inline everything:

ProfiledApp.setSnapshot({ result: useReadQuery(queryRef) })

I'd like to preserve some way of being able to partial update a key on the snapshot without the use of a callback function if possible to cut down on the noise. I do agree that the immer-style way of doing things is useful for incrementing counters or "mutating" existing values. Perhaps there is a way we can have both?

See the comment above - we might also consider the current behaviour, but immer-wrapped.